-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#53] Add section for reference frame #62
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good changes overall, might have to standardise some things
tutorials/plantUml.md
Outdated
<tr class="odd"> | ||
<td> | ||
<pre> | ||
ref over MSLogic, TextUi : get minefield appearance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment would be helping to tell students that this is for the reference frame only.
OR
Moreover, it might be better to simplify the image. It could focus on the section between :TextUi
and :MsLogic
. Then, include the entire code within the Source block, similar to how the full code for the overall image in above sections
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the suggestion!
I have went ahead and simplified the diagrams as much I could. I have not simplified it fully as I was concerned that it could undermine the example's relevance. If the diagrams are too simplified, it might be harder to see and connect the usage of reference frames in an actual use case.
Could you suggest any specific areas that could be removed, or do you think the current diagrams are sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, simple and clear implementation of the tip!
@mfjkri Let's wrap this up. |
@mfjkri Let me know when this is ready for review again. |
@damithc Yup its ready for review again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the changes. LGTM!
Fixes #51